- 
                Notifications
    
You must be signed in to change notification settings  - Fork 516
 
WWSTCERT-8357/8360 Inovelli VZW32-SN: Adding support for this device for the WWST program #2371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
WWSTCERT-8357/8360 Inovelli VZW32-SN: Adding support for this device for the WWST program #2371
Conversation
…mware update process during certification
| 
           Duplicate profile check: Passed - no duplicate profiles detected.  | 
    
| 
           Invitation URL:  | 
    
          Test Results   71 files    462 suites   0s ⏱️ Results for commit 4742d35. ♻️ This comment has been updated with latest results.  | 
    
| 
           
 
 Minimum allowed coverage is  Generated by 🐒 cobertura-action against 4742d35  | 
    
| - id: colorTemperature | ||
| version: 1 | ||
| config: | ||
| values: | ||
| - key: "colorTemperature.value" | ||
| range: [ 2700, 6500 ] | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can now simply define the range your bulb supports in code using the colorTemperatureRange attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@greens
Should I do something like this during device initialization then?
device:emit_event(capabilities.colorTemperature.colorTemperatureRange({ value = {minimum = 2700, maximum = 6500} }))
device:emit_event(capabilities.colorTemperature.colorTemperature(6500))
Then, should I change the profile to rgbw-bulb.yml and take out the config section?
| - id: firmwareUpdate | ||
| version: 1 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're not emitting any events using this capability, please omit it.
If we do add OTA support for z-wave in the future, we will handle adding it to device profiles as necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this capability.
        
          
                drivers/SmartThings/zwave-switch/src/inovelli-vzw32-sn/init.lua
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                drivers/SmartThings/zwave-switch/src/inovelli-vzw32-sn/init.lua
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                drivers/SmartThings/zwave-switch/src/inovelli-vzw32-sn/init.lua
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a significant number of unit tests. We like our unit test code coverage to be >90%. Yours is currently at 32%.
          
 For some reason I can't get my unit tests to work. It is like they aren't loading the driver at all. I'm getting 0/7 for example, but the driver works perfectly in practice and through the certification test suite. Unit test failures in C:\SmartThingsEdgeDrivers\drivers\SmartThings\zwave-switch\src\test\test_inovelli_vzw32_sn.lua: I've spent a few hours on it today and can't get it working. I can try more tomorrow.  | 
    
| 
           @InovelliUSA that's because there's an implied  Alternatively, you could restructure your driver so that the events generated in   | 
    
| local device_init = function(self, device) | ||
| if device.network_type ~= st_device.NETWORK_TYPE_CHILD then | ||
| device:set_component_to_endpoint_fn(component_to_endpoint) | ||
| device:set_endpoint_to_component_fn(endpoint_to_component) | ||
| --initialize(device) | ||
| end | ||
| end | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should no longer need to override the base driver's init function now (or the endpoint/component mapping functions)
| 
           I just noticed my motion sensor isn't initializing for the main device. I thought "refresh" would catch it, but I can add it explicitly on Monday.  | 
    
| 
           @InovelliUSA our defaults for motion sensor rely on the   | 
    
          
 We are using the NOTIFICATION command class.  | 
    
| 
           @InovelliUSA Then I would suggest overwriting the refresh handler with a new function that first calls   | 
    
          
 Something strange is happening when I do this. The process is working fine on the Zigbee driver, but the Z-wave driver becomes unresponsive when I use "refresh". I have tested a few different variants. Even this one causes the driver to stop responding: local function refresh_handler(driver, device) I will sometimes this in the logs after some time of hitting the refresh button: 2025-10-29T21:10:35.357313398Z TRACE Z-Wave Switch  Found CapabilityCommandDispatcher handler in zwave_switch -> inovelli vzw32-sn handler  | 
    
| 
           @InovelliUSA that was my mistake. Z-Wave and Zigbee handle  The fix is to use  Also I noticed the same thing with the z-wave version of the vzw-31 that we saw with zigbee, so I'd ask you to consolidate here as well. Good news is that there is already a parent driver: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/tree/main/drivers/SmartThings/zwave-switch/src/inovelli-LED  | 
    
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests